Clean up legacy test scaffolding in cudf-polars by madsbk · Pull Request #22535 · rapidsai/cudf

madsbk · 2026-05-16T09:06:27Z

Description

Removes assert_collect_raises, its callers, and the leftover executor=-era keyword plumbing that is no longer used anywhere else.

madsbk · 2026-05-16T09:06:44Z

    engine: GPUEngine,
    collect_kwargs: CollectKwargs | None = None,
    polars_collect_kwargs: CollectKwargs | None = None,
-    cudf_collect_kwargs: CollectKwargs | None = None,


madsbk · 2026-05-16T09:07:09Z

-def _process_kwargs(
-    collect_kwargs: CollectKwargs | None,
-    polars_collect_kwargs: CollectKwargs | None,
-    cudf_collect_kwargs: CollectKwargs | None,
-) -> tuple[CollectKwargs, CollectKwargs]:
-    if collect_kwargs is None:
-        collect_kwargs = {}
-    final_polars_collect_kwargs = collect_kwargs.copy()
-    final_cudf_collect_kwargs = collect_kwargs.copy()
-    if polars_collect_kwargs is not None:  # pragma: no cover; not currently used
-        final_polars_collect_kwargs.update(polars_collect_kwargs)
-    if cudf_collect_kwargs is not None:  # pragma: no cover; not currently used
-        final_cudf_collect_kwargs.update(cudf_collect_kwargs)
-    return final_polars_collect_kwargs, final_cudf_collect_kwargs


Inlining, the function is only called once now.

madsbk · 2026-05-16T09:07:52Z

-    return final_polars_collect_kwargs, final_cudf_collect_kwargs
-
-
-def assert_collect_raises(


Inlining the function is simpler to understand IMO. We now just do:

- assert_collect_raises( - q, - cudf_except=pl.exceptions.ComputeError, - polars_except=pl.exceptions.InvalidOperationError, - ) + with pytest.raises(pl.exceptions.InvalidOperationError): + q.collect() + with pytest.raises(pl.exceptions.ComputeError): + q.collect(engine=pl.GPUEngine(executor="in-memory", raise_on_fail=True))

coderabbitai · 2026-05-16T10:39:05Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 79322a23-9a30-4ec1-b96f-dfbb03a02c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 0c15199 and 2c35ce4.

📒 Files selected for processing (1)

python/cudf_polars/cudf_polars/testing/asserts.py

🚧 Files skipped from review as they are similar to previous changes (1)

python/cudf_polars/cudf_polars/testing/asserts.py

📝 Walkthrough

Summary by CodeRabbit

Refactor
- Simplified internal testing utility by removing redundant parameter and consolidating collection kwargs handling.
- Replaced assertion helper with explicit exception checking for improved test clarity and maintainability.
Tests
- Updated test cases to use direct exception assertions instead of removed utility function.

Walkthrough

This PR simplifies the cuDF Polars test assertion API by removing the cudf_collect_kwargs parameter from assert_gpu_result_equal, merging CPU/GPU kwargs logic, and deleting the assert_collect_raises helper. Tests are updated to use explicit pytest.raises for collection-time errors.

Changes

Assertion helper refactor and test migration

Layer / File(s)	Summary
Assertion utilities refactoring `python/cudf_polars/cudf_polars/testing/asserts.py`	`assert_gpu_result_equal` removes `cudf_collect_kwargs` parameter; CPU kwargs are built by merging `collect_kwargs` with `polars_collect_kwargs`, GPU kwargs come from `collect_kwargs` alone. Private helper `_process_kwargs` and public helper `assert_collect_raises` are deleted.
Test migration to direct pytest.raises `python/cudf_polars/tests/expressions/test_casting.py`, `python/cudf_polars/tests/expressions/test_datetime_basic.py`, `python/cudf_polars/tests/expressions/test_stringfunction.py`, `python/cudf_polars/tests/testing/test_asserts.py`	Test modules remove `assert_collect_raises` imports and replace prior helper usage with explicit `pytest.raises(...)` blocks around both `q.collect()` and `q.collect(engine=pl.GPUEngine(..., raise_on_fail=True))`. The standalone `test_collect_assert_raises()` test is removed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

rapidsai/cudf#22493: Both PRs refactor asserts.py's assertion utilities and engine-related calling conventions in assert_gpu_result_equal.

Suggested labels

non-breaking

Suggested reviewers

mroeschke
wence-

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main objective of the PR: removing legacy test scaffolding (`assert_collect_raises`) and related keyword plumbing from the cudf-polars testing module.
Description check	✅ Passed	The description is directly related to the changeset, clearly explaining what is being removed: the `assert_collect_raises` utility, its callers, and outdated `executor=` keyword parameters.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Co-authored-by: Lawrence Mitchell <wence@gmx.li>

madsbk · 2026-05-18T18:45:33Z

/merge

PR rapidsai#22048 ("Bump polars upper bound to <1.40", merged today) added the new `test_hconcat_strict_different_heights` test, which imports `assert_collect_raises`. However, PR rapidsai#22535 ("Clean up legacy test scaffolding", also merged into `release/26.06`) removed that helper. The two PRs landed on `release/26.06` without the conflict being noticed. On `main`, `test_hconcat.py` does not contain the strict-mode test, so the issue is limited to `release/26.06`.

#22558) PR #22048 (merged today) added the new `test_hconcat_strict_different_heights` test, which imports `assert_collect_raises`. However, PR #22535 (also merged today) removed that helper. The two PRs landed on `release/26.06` without the conflict being noticed. On `main`, `test_hconcat.py` does not contain the strict-mode test, so the issue is limited to `release/26.06`. Authors: - Mads R. B. Kristensen (https://github.com/madsbk) Approvers: - Matthew Murray (https://github.com/Matt711) URL: #22558

Removes `assert_collect_raises`, its callers, and the leftover `executor=`-era keyword plumbing that is no longer used anywhere else. Authors: - Mads R. B. Kristensen (https://github.com/madsbk) Approvers: - Lawrence Mitchell (https://github.com/wence-) - Matthew Roeschke (https://github.com/mroeschke) URL: rapidsai#22535

rapidsai#22558) PR rapidsai#22048 (merged today) added the new `test_hconcat_strict_different_heights` test, which imports `assert_collect_raises`. However, PR rapidsai#22535 (also merged today) removed that helper. The two PRs landed on `release/26.06` without the conflict being noticed. On `main`, `test_hconcat.py` does not contain the strict-mode test, so the issue is limited to `release/26.06`. Authors: - Mads R. B. Kristensen (https://github.com/madsbk) Approvers: - Matthew Murray (https://github.com/Matt711) URL: rapidsai#22558

cudf-polars: remove legacy test code utils

0c15199

madsbk self-assigned this May 16, 2026

madsbk added improvement Improvement / enhancement to an existing function breaking Breaking change labels May 16, 2026

github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels May 16, 2026

github-project-automation Bot added this to cuDF Python May 16, 2026

madsbk commented May 16, 2026

View reviewed changes

GPUtester moved this to In Progress in cuDF Python May 16, 2026

madsbk commented May 16, 2026

View reviewed changes

madsbk mentioned this pull request May 16, 2026

Cleanup the legacy engines code path #22488

Closed

madsbk marked this pull request as ready for review May 16, 2026 10:36

madsbk requested a review from a team as a code owner May 16, 2026 10:36

madsbk requested a review from nirandaperera May 16, 2026 10:36

rapidsai deleted a comment from copy-pr-bot Bot May 16, 2026

This comment was marked as outdated.

Sign in to view

wence- approved these changes May 18, 2026

View reviewed changes

Comment thread python/cudf_polars/cudf_polars/testing/asserts.py Outdated

madsbk and others added 2 commits May 18, 2026 16:26

Apply suggestions from code review

a3da1a2

Co-authored-by: Lawrence Mitchell <wence@gmx.li>

Merge branch 'release/26.06' into cleanup_legacy_tests

2c35ce4

mroeschke reviewed May 18, 2026

View reviewed changes

Comment thread python/cudf_polars/tests/expressions/test_casting.py

mroeschke approved these changes May 18, 2026

View reviewed changes

rapids-bot Bot merged commit 02f1051 into rapidsai:release/26.06 May 18, 2026
87 checks passed

github-project-automation Bot moved this from In Progress to Done in cuDF Python May 18, 2026

madsbk deleted the cleanup_legacy_tests branch May 18, 2026 18:46

madsbk mentioned this pull request May 18, 2026

[release/26.06] Remove stale import of deleted assert_collect_raises #22558

Merged

mroeschke mentioned this pull request May 18, 2026

Excercise assert_collect_raises with engine fixture #22563

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up legacy test scaffolding in cudf-polars#22535

Clean up legacy test scaffolding in cudf-polars#22535
rapids-bot[bot] merged 3 commits into
rapidsai:release/26.06from
madsbk:cleanup_legacy_tests

madsbk commented May 16, 2026

Uh oh!

madsbk May 16, 2026

Uh oh!

madsbk May 16, 2026

Uh oh!

madsbk May 16, 2026

Uh oh!

coderabbitai Bot commented May 16, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

madsbk commented May 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		return final_polars_collect_kwargs, final_cudf_collect_kwargs


		def assert_collect_raises(

Conversation

madsbk commented May 16, 2026

Description

Uh oh!

madsbk May 16, 2026

Choose a reason for hiding this comment

Uh oh!

madsbk May 16, 2026

Choose a reason for hiding this comment

Uh oh!

madsbk May 16, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

madsbk commented May 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented May 16, 2026 •

edited

Loading